Skip to content

Fix light mode serial console#3127

Merged
david-crespo merged 6 commits intomainfrom
light-mode-serial-console
Mar 16, 2026
Merged

Fix light mode serial console#3127
david-crespo merged 6 commits intomainfrom
light-mode-serial-console

Conversation

@benjaminleonard
Copy link
Contributor

@benjaminleonard benjaminleonard commented Mar 16, 2026

Colours in light mode aren't super saturated. Can possibly revisit and add different ones per mode but doesn't seem super important. I'd have to review other peoples light themes to see how they use colour but keep the contrast high.

CleanShot 2026-03-16 at 15 51 36

#3087

@vercel
Copy link

vercel bot commented Mar 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
console Ready Ready Preview Mar 16, 2026 10:43pm

Request Review


window.addEventListener('resize', resize)

// Update terminal colors when the theme changes
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small thing but otherwise you're stuck with the alternate theme if you switch on this screen

// Ensure the connecting skeleton shows for at least 1s so users see
// the connection is being established rather than a brief flash
const connectingAt = useRef(canConnect ? Date.now() : 0)
const MIN_CONNECTING_MS = 1000
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flash looked a little broken if it came through quickly

ws.current?.addEventListener('error', setError)

return () => {
clearTimeout(openTimer)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of React is my kryptonite @david-crespo please review

@benjaminleonard benjaminleonard added this to the 19 milestone Mar 16, 2026
await expect(xterm.getByText('Wake up Neo...').first()).toBeVisible()
// MSW streams a boot log. Wait for the login prompt which comes at the end.
await expect(xterm.getByText('oxide-instance login:').first()).toBeVisible({
timeout: 15000,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels bad to waste this much time on the test?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine in theory, but it did flake in CI, so I'll see if we can avoid it.

@david-crespo
Copy link
Collaborator

david-crespo commented Mar 16, 2026

Light mode contrast seems good to me. The colors are not very readable as colors but I can live with it for the release.

@david-crespo
Copy link
Collaborator

Unfortunately this doesn't test well in local dev pointed at dogfood, so I will merge this and test it by deploying to dogfood.

Copy link
Collaborator

@david-crespo david-crespo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. MutationObserver is good. Got rid of the delay on open state because in production there's a delay on connection that's hard to simulate in MSW.

@david-crespo
Copy link
Collaborator

Worked well on dogfood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants